Add BAD descriptor to xfeatures2d module#3277
Conversation
|
A missing file with pre-computed bad descriptors caused the tests to fail. I added it in a opencv_extra fork and made the necessary pull request. It should be fixed now. I would really appreciate it if you could rerun the GitHub actions. |
alalek
left a comment
There was a problem hiding this comment.
Thank you for the great contribution!
Please take a look on comments below.
| { | ||
| int x1, y1, x2, y2, boxRadius, th; | ||
| int x1, y1, x2, y2, boxRadius; | ||
| float th; |
There was a problem hiding this comment.
We should keep integer-based computation of original BEBLID.
Could you please add separate struct type with floats? (and change BEBLID computation to use params from template type)
There was a problem hiding this comment.
Thank you very much @alalek for your feedback.
You are right. It is a better idea to preserve the original structure for BEBLID. In my last commit, I added a new ABWLParamsFloatTh for the new descriptor and undo the changes to the original ABWLParams.
I also made BEBLID_Impl a templated class that can be used with both structs.
|
|
||
| The descriptor was trained in Liberty split of the UBC datasets \cite winder2007learning . | ||
| */ | ||
| class CV_EXPORTS_W BAD : public Feature2D |
There was a problem hiding this comment.
BTW, BAD is not a good name for googling. Could we add here some prefix/suffix?
There was a problem hiding this comment.
Done.
After discussing with the other authors, we think that a better name would be TEBLID (Triplet-based Efficient Binary Local Image Descriptor), which makes it clear that it is an improvement over BEBLID and the name is better suited for googling. In any case, in the documentation, we indicate that this descriptor appears as BAD in the paper.
| It is able to describe keypoints from any detector just by changing the scale_factor parameter. | ||
| TEBLID is as efficient as ORB, BEBLID or BRISK, but the triplet-based training objective selected more | ||
| discriminative features that explain the accuracy gain. It is also more compact than BEBLID, | ||
| when running the [AKAZE example](https://raw.githubusercontent.com/opencv/opencv/master/samples/cpp/tutorial_code/features2D/AKAZE_match.cpp) |
There was a problem hiding this comment.
Could you please use this link? https://github.com/opencv/opencv/blob/4.x/samples/cpp/tutorial_code/features2D/AKAZE_match.cpp
- master -> 4.x
- use GitHub UI instead of raw code (some browsers open "download" UI)
modules/xfeatures2d/src/beblid.cpp
Outdated
| if (n_bits == BEBLID::SIZE_512_BITS) | ||
| { | ||
| #include "beblid.p512.hpp" | ||
| begin_ptr = (WeakLearnerT *) std::begin(beblid_wl_params_512); | ||
| end_ptr = (WeakLearnerT *) std::end(beblid_wl_params_512); |
There was a problem hiding this comment.
(WeakLearnerT *)
Using WeakLearnerT = ABWLParamsFloatTh and BEBLID::SIZE_512_BITS) is not a valid case and we should emit error.
Probably it makes sense to specialize ctor and/or add wl_params parameter.
template <class WeakLearnerT>
BEBLID_Impl<WeakLearnerT>::BEBLID_Impl(float scale_factor, int n_bits, std::vector<WeakLearnerT>&& wl_params)
and move parameters vector initialization on upper (caller) level (::create() calls).
Also these changes would not require special values like 102,103 for enum values.
There was a problem hiding this comment.
Thank you very much again for your feedback @alalek .
I implemented your suggestion in my last commit. By doing it we have one extra benefit, the enum with the descriptor size is no longer needed in the constructor. Also, I decided to use a const reference ( const std::vector<WeakLearnerT>& wl_params ) instead of a Rvalue reference because in this way we can keep the weights of the .h files constant and only one copy of the vector is done.
| // Pre-trained parameters of TEBLID-256 trained in Liberty data set. 10K triplets are sampled per iteration. Each triplet | ||
| // contains an anchor patch, a positive and a negative, selected as the hardest among 256 random negatives. | ||
| static const std::vector<ABWLParamsFloatTh> teblid_wl_params_256 = { | ||
| {25, 14, 13, 15, 6, 21.65}, {16, 15, 14, 11, 1, 5.65}, {14, 14, 7, 8, 6, 4.95}, |
There was a problem hiding this comment.
Could you please add f suffixes to floating-point numbers?
-21.65
+21.65fIt is necessary to resolve build problem on Windows:
c:\build\precommit-contrib_windows64\4.x\opencv_contrib\modules\xfeatures2d\src\teblid.p512.hpp(187): error C2397: conversion from 'double' to 'float' requires a narrowing conversion (compiling source file C:\build\precommit-contrib_windows64\4.x\opencv_contrib\modules\xfeatures2d\src\beblid.cpp)
|
|
||
| // Pre-trained parameters of TEBLID-256 trained in Liberty data set. 10K triplets are sampled per iteration. Each triplet | ||
| // contains an anchor patch, a positive and a negative, selected as the hardest among 256 random negatives. | ||
| static const std::vector<ABWLParamsFloatTh> teblid_wl_params_256 = { |
There was a problem hiding this comment.
std::vector
Need to check generated binary code of this initializer. Array should be fine, but std::vector - doesn't.
Could you please use something like this:
static const ABWLParamsFloatTh teblid_wl_params_256_[] = {...};
static const std::vector<ABWLParamsFloatTh> teblid_wl_params_256(teblid_wl_params_256_, teblid_wl_params_256 + sizeof(teblid_wl_params_256_) / sizeof(teblid_wl_params_256_[0]));
There was a problem hiding this comment.
Sorry for late reply due to vacations and thanks again for your comments.
I have fixed it in my last commit.
alalek
left a comment
There was a problem hiding this comment.
Well done! Thank you for contribution 👍
|
Thank you @alalek for your support and for maintaining OpenCV! It's a wonderful library, and I am always happy to contribute 😃 |
Add BAD descriptor to xfeatures2d module * Adding new BAD descriptor to xfeatures2d module * Changing BAD name by TEBLID and using int threshold again for BEBLID * Changing link to AKAZE tutorial and moved parameters initialization to ::create() * Adding f suffixes to floating-point parameters and using arrays again
Merge with extra: opencv/opencv_extra#978
Box Average Difference (BAD) is a new descriptor published last year. The method improves efficient binary descriptors such as ORB or BEBLID with a new loss function and optimization method. It outperforms the other binary efficient descriptors in several tasks like image matching, structure from motion, or patch retrieval. It is also compact, being able to outperform BEBLID-512 with only 256 bits. Here we can see a comparison in HPatches dataset from the paper:
Since the implementation is quite similar to the one of BEBLID, the same source code can be used. It is only necessary to add the new pre-trained weights and some minor modifications. The original BAD source can be found here. More details in the paper and the project page.
I tried to make the code clear, tested, and documented. If you find some possible improvements, please just let me know.
Pull Request Readiness Checklist
Patch to opencv_extra has the same branch name.